Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Broadcasting for semiclassical objects #404

Merged
merged 13 commits into from
Aug 11, 2024
Merged

Conversation

apkille
Copy link
Contributor

@apkille apkille commented Jul 30, 2024

@david-pl @Krastanov @amilsted

  • Added an interface for broadcasting State objects, which enables use of DiffEq solvers
  • With these changes, ODE problems for States can be naturally written as the following example:
using QuantumOptics, OrdinaryDiffEq, BenchmarkTools

b = SpinBasis(1//2)
psi0 = spindown(b)
u0 = ComplexF64[0.5, 0.75] 
sc = semiclassical.State(psi0, u0)
t₀, t₁ = (0.0, pi)
const σx = sigmax(b)

fquantum(t, q, u) = σx + cos(u[1])*identityoperator(σx)
fclassical!(du, u, q, t) = (du[1] = sin(u[2]); du[2] = 2*u[1])
f!(dstate, state, p, t) = semiclassical.dschroedinger_dynamic!(dstate, fquantum, fclassical!, state, t)
prob = ODEProblem(f!, sc, (t₀, t₁))

julia> @benchmark sol = solve($prob, DP5(); save_everystep=false)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  50.166 μs … 133.706 ms  ┊ GC (min … max):  0.00% … 99.93%
 Time  (median):     52.583 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   70.621 μs ±   1.339 ms  ┊ GC (mean ± σ):  23.35% ±  3.73%

       ▃█▅                                                      
  ▁▂▃▆████▇▃▂▂▂▂▂▂▂▂▂▃▃▃▃▂▃▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  50.2 μs         Histogram: frequency by time         69.8 μs <

 Memory estimate: 93.81 KiB, allocs estimate: 1136.

julia> @benchmark semiclassical.schroedinger_dynamic([$t₀, $t₁], $sc, $fquantum, $fclassical!)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  117.875 μs … 137.310 ms  ┊ GC (min … max):  0.00% … 99.87%
 Time  (median):     122.959 μs               ┊ GC (median):     0.00%
 Time  (mean ± σ):   148.349 μs ±   1.379 ms  ┊ GC (mean ± σ):  15.47% ±  6.25%

    ▃█▇▁▁▃▃                                                      
  ▂▅████████▄▃▃▂▂▃▃▂▃▂▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  118 μs           Histogram: frequency by time          163 μs <

 Memory estimate: 225.66 KiB, allocs estimate: 2637.
  • Support is provided for both kets and density operators.
  • Note: broadcasting for a custom type like State is weird, and really the only reason we're doing it is to have interoperability of semiclassical objects with SciML.
  • Some of these interface decisions probably can be debated. I based most of them off of how State objects were recasted in the semiclassical.schroedinger_dynamic, semiclassical.master_dynamic, etc. functions.

Copy link
Collaborator

@Krastanov Krastanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to merge after a small import change.

src/semiclassical.jl Outdated Show resolved Hide resolved
@Krastanov
Copy link
Collaborator

And after the errors in the tests are fixed. Does this depend on the changes to QuantumOpticsBase in its broadcast PR? If so, we should bump the compat bound for it.

@Krastanov
Copy link
Collaborator

And same comment as in the QuantumOpticsBase PR on adding your examples from the PR description to the test runner, maybe as a new file.

@apkille
Copy link
Contributor Author

apkille commented Jul 31, 2024

And after the errors in the tests are fixed. Does this depend on the changes to QuantumOpticsBase in its broadcast PR? If so, we should bump the compat bound for it.

Yes, so we should merge this after we merge the QuantumOpticsBase PR.

@apkille
Copy link
Contributor Author

apkille commented Jul 31, 2024

@Krastanov Should be good to go.

Copy link

codecov bot commented Aug 10, 2024

Codecov Report

Attention: Patch coverage is 86.36364% with 9 lines in your changes missing coverage. Please review.

Project coverage is 96.66%. Comparing base (d2c71fe) to head (bb2d0f5).
Report is 11 commits behind head on master.

Files Patch % Lines
src/semiclassical.jl 86.36% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #404      +/-   ##
==========================================
- Coverage   97.03%   96.66%   -0.38%     
==========================================
  Files          18       19       +1     
  Lines        1554     1678     +124     
==========================================
+ Hits         1508     1622     +114     
- Misses         46       56      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@apkille
Copy link
Contributor Author

apkille commented Aug 10, 2024

@Krastanov is there anything preventing this from getting merged?

Copy link
Collaborator

@Krastanov Krastanov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me.

There is a minor issue with isapprox not propagating its kwargs.

More importantly though, there are a few functions that codecov reports as not covered by tests. Could you add new tests for them, just to ensure the review is not missing anything? I am a bit skittish here as this touches a lot of basic interfaces.

src/semiclassical.jl Outdated Show resolved Hide resolved
@apkille
Copy link
Contributor Author

apkille commented Aug 11, 2024

@Krastanov good catch. Should be covered now.

@Krastanov Krastanov merged commit beb0f37 into qojulia:master Aug 11, 2024
5 of 7 checks passed
@apkille apkille deleted the semiclass branch August 11, 2024 09:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants